Skip to content

Fix/windows conpty ansi leak - #857

Open
ilyaxuwu wants to merge 2 commits into
CodebuffAI:mainfrom
ilyaxuwu:fix/windows-conpty-ansi-leak
Open

Fix/windows conpty ansi leak#857
ilyaxuwu wants to merge 2 commits into
CodebuffAI:mainfrom
ilyaxuwu:fix/windows-conpty-ansi-leak

Conversation

@ilyaxuwu

@ilyaxuwu ilyaxuwu commented Jul 9, 2026

Copy link
Copy Markdown

No description provided.

@ilyaxuwu

ilyaxuwu commented Jul 9, 2026

Copy link
Copy Markdown
Author

Hopefully this fixes Codebuff/Freebuff!

@jahooma

jahooma commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Thanks! Coding agent thought it was a reasonable fix. Merged a version of this

@codebuff-team

Copy link
Copy Markdown
Contributor

The instinct here is reasonable — unifying process-group behavior across platforms so grandchild processes get killed consistently on timeout/abort. But the justification in the new comment is not accurate: Node's detached on Windows is implemented via libuv, which does not map to DETACHED_PROCESS. In src/win/process.c, UV_PROCESS_DETACHED sets CREATE_NEW_PROCESS_GROUP | CREATE_BREAKAWAY_FROM_JOB, not DETACHED_PROCESS. So the console-attachment story in the comment (detached: true + windowsHide = fully detached console) doesn't hold up against the actual implementation.

That matters here because the original code deliberately set detached: !isWindows specifically to avoid the ConPTY ANSI-leak bug this PR claims to fix. Reverting that with an incorrect rationale risks reintroducing the exact bug it's supposed to fix, or at minimum changes signal/kill semantics on Windows (new process group via CREATE_NEW_PROCESS_GROUP) without any test or repro showing the leak is actually gone.

Given the PR is only a comment + one flag change with no test coverage, no repro steps, and no evidence the ANSI-leak scenario was actually verified on Windows post-change, this needs more rigor before it can be ported: either a reproduction script/test demonstrating the leak is fixed with detached: true, or corrected reasoning that reflects libuv's actual Windows behavior (e.g., is it windowsHide/CREATE_NO_WINDOW alone doing the real work, and is detached orthogonal?).

@codebuff-team codebuff-team added bot:triaged Classified by the community triage bot pr:needs-work Right idea, not mergeable as written labels Aug 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bot:triaged Classified by the community triage bot pr:needs-work Right idea, not mergeable as written

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants